[homeassistant] Upgrade to Graal 25#19518
Conversation
|
@HolgerHees @florian-h05: we should probably prepare all three add-ons to work with Graal 25, and submit the PRs together. I just don't know how well things will interact with having multiple versions of Graal in the OSGi process with how we've had to wrap the service lookups. Also, FYI openhab/openhab-osgiify#79 should be available soon (it's failing the build because the release just happened, and it looks like all artifacts aren't available yet). |
|
I fully agree. Starting this weekend, I will spend time into it. |
|
|
||
| <properties> | ||
| <graalpy.version>25.0.0</graalpy.version> | ||
| <graalpy.version>25.0.1</graalpy.version> |
There was a problem hiding this comment.
you can use "${graalpy.version}" here
There was a problem hiding this comment.
hmm.. this sets the value of that variable.
There was a problem hiding this comment.
ahh, sorry my mistake, I want to say, that you must not define it here. There is already an approach of having a more global variable
Line 78 in 1aca2fb
There was a problem hiding this comment.
I would suggest renaming the global variable to graal.version, so I can use it for JS Scripting as well 👍
There was a problem hiding this comment.
is changed as part of my current pull request
There was a problem hiding this comment.
Please adapt to ${graalvm.version}
There was a problem hiding this comment.
I rebased on top of #19568, and then switched to graalvm.version. Once that PR merges, I'll rebase again to resolve merge "conflicts" from that PR getting squashed.
|
@ccutrer @florian-h05 just a first impression from pythonscripting upgrade to graalvm 25. No noticeable impact. but I will test next days a bit more. |
|
GraalJS changelog is fairly short, everything looks good. |
|
JS Scripting PR is open: #19567 |
|
Python Scripting PR is open: #19568 |
|
@ccutrer as requested by @florian-h05 , I renamed the global "graalpy.version" property to "graalvm.version" I had also to fix the usage in itests of mqtt.homeassistant.tests. I hope it is ok. otherwise the build would fail. |
|
@ccutrer, @florian-h05 as milstone2 is out. Lets merge :-) |
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades GraalVM Python dependencies from version 24.2.1 to 25.0.1 and replaces the external voluptuous Python package (previously installed via pip) with an embedded fork of the library included directly in the source tree.
Key changes:
- GraalVM Python version upgraded from 24.2.1 to 25.0.1 across all configuration files
- Voluptuous library (version 0.15.2) source code added to
src/main/python/voluptuous/directory - Voluptuous removed from pip package dependencies in pom.xml
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 21 comments.
Show a summary per file
| File | Description |
|---|---|
| itests/.../itest.bndrun | Updated GraalVM dependency version ranges from 24.2.1 to 25.0.1 |
| features/.../footer.xml | Updated GraalVM Maven bundle versions from 24.2.1 to 25.0.1 |
| bundles/.../feature.xml | Updated GraalVM Maven bundle versions from 24.2.1 to 25.0.1 |
| bundles/.../pom.xml | Updated graalpy.version property to 25.0.1 and removed voluptuous from pip packages |
| bundles/.../NOTICE | Added attribution for forked voluptuous library |
| bundles/.../python/voluptuous/*.py | Added complete voluptuous library source code (5 modules) |
Comments suppressed due to low confidence (4)
bundles/org.openhab.binding.mqtt.homeassistant/src/main/python/voluptuous/schema_builder.py:1
- The union type syntax using
|requires Python 3.10+. For better compatibility with older Python versions, usetyping.Union[SupportsAllComparisons, None]ortyping.Optional[SupportsAllComparisons]instead.
# fmt: off
bundles/org.openhab.binding.mqtt.homeassistant/src/main/python/voluptuous/schema_builder.py:1
- The union type syntax using
|requires Python 3.10+. For better compatibility with older Python versions, usetyping.Union[SupportsAllComparisons, None]ortyping.Optional[SupportsAllComparisons]instead.
# fmt: off
bundles/org.openhab.binding.mqtt.homeassistant/src/main/python/voluptuous/schema_builder.py:1
- The union type syntax using
|requires Python 3.10+. For better compatibility with older Python versions, usetyping.Union[SupportsAllComparisons, None]ortyping.Optional[SupportsAllComparisons]instead.
# fmt: off
bundles/org.openhab.binding.mqtt.homeassistant/src/main/python/voluptuous/schema_builder.py:1
- The union type syntax using
|requires Python 3.10+. For better compatibility with older Python versions, usetyping.Union[typing.Container, typing.Iterable]instead.
# fmt: off
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self, | ||
| schema_: Schemable, | ||
| msg: typing.Optional[str] = None, | ||
| description: typing.Any | None = None, |
There was a problem hiding this comment.
The union type syntax using | requires Python 3.10+. For better compatibility with older Python versions, use typing.Optional[typing.Any] instead.
| description: typing.Any | None = None, | |
| description: typing.Optional[typing.Any] = None, |
| schema: Schemable, | ||
| msg: typing.Optional[str] = None, | ||
| default: typing.Any = UNDEFINED, | ||
| description: typing.Any | None = None, |
There was a problem hiding this comment.
The union type syntax using | requires Python 3.10+. For better compatibility with older Python versions, use typing.Optional[typing.Any] instead.
| schema: Schemable, | ||
| group_of_exclusion: str, | ||
| msg: typing.Optional[str] = None, | ||
| description: typing.Any | None = None, |
There was a problem hiding this comment.
The union type syntax using | requires Python 3.10+. For better compatibility with older Python versions, use typing.Optional[typing.Any] instead.
| description: typing.Any | None = None, | |
| description: typing.Optional[typing.Any] = None, |
| schema: Schemable, | ||
| group_of_inclusion: str, | ||
| msg: typing.Optional[str] = None, | ||
| description: typing.Any | None = None, |
There was a problem hiding this comment.
The union type syntax using | requires Python 3.10+. For better compatibility with older Python versions, use typing.Optional[typing.Any] instead.
| description: typing.Any | None = None, | |
| description: typing.Optional[typing.Any] = None, |
| schema: Schemable, | ||
| msg: typing.Optional[str] = None, | ||
| default: typing.Any = UNDEFINED, | ||
| description: typing.Any | None = None, |
There was a problem hiding this comment.
The union type syntax using | requires Python 3.10+. For better compatibility with older Python versions, use typing.Optional[typing.Any] instead.
| self.default = default_factory(default) | ||
|
|
||
|
|
||
| class Remove(Marker): |
There was a problem hiding this comment.
| try: | ||
| validate(path, value) | ||
| break | ||
| except er.Invalid: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except er.Invalid: | |
| except er.Invalid: | |
| # Ignore and try the next validator; only error if all validators fail. |
|
|
||
| def _exec(self, funcs, v, path=None): | ||
| error = None | ||
| for func in funcs: |
There was a problem hiding this comment.
This 'for' statement has a redundant 'else' as no 'break' is present in the body.
|
|
||
| def _exec(self, funcs, v, path=None): | ||
| error = None | ||
| for func in funcs: |
There was a problem hiding this comment.
This 'for' statement has a redundant 'else' as no 'break' is present in the body.
ccutrer
force-pushed
the
homeassistant-graal-25
branch
from
66b238b to
57bb2c5
Compare
|
when rebasing, could you also lookinto the co-pilot review? |
Signed-off-by: Cody Cutrer <cody@cutrer.us>
Signed-off-by: Cody Cutrer <cody@cutrer.us>
Signed-off-by: Cody Cutrer <cody@cutrer.us>
ccutrer
force-pushed
the
homeassistant-graal-25
branch
from
57bb2c5 to
b6fd604
Compare
|
Rebased. The copilot comments are all in the imported voluptuous code, so I don't intend to address them. The upstream bug in Graal has been fixed, so on the next Graal update the voluptuous code will be removed and we'll go back to using the packaged version of it. |
ccutrer
added
the
enhancement
* [homeassistant] Upgrade to Graal 25.01 Signed-off-by: Cody Cutrer <cody@cutrer.us> Signed-off-by: Erik De Boeck <deboeck.erik@gmail.com>
4 participants
Unfortunately, due to oracle/graalpython#559, I had to import Voluptuous completely, and modify it slightly to work around that bug. Other than that, all tests passed.